Skip to content

rustc_codegen_llvm: Tidying of update_target_reliable_float_cfg #146747

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
a4lg wants to merge 3 commits into
base: main
Choose a base branch
from
Open

rustc_codegen_llvm: Tidying of update_target_reliable_float_cfg #146747

a4lg wants to merge 3 commits into from
+11 −11

Conversation

@a4lg
Copy link
Contributor

@a4lg a4lg commented Sep 19, 2025 •
edited
Loading

This PR simplifies floating type handling through update_target_reliable_float_cfg based on several facts:

  1. Major changes in behavior normally occurs only on the major LLVM upgrade.
  2. The first release of LLVM 20.x.x is 20.1.0.

Due to the first fact, we can normally ignore minor and patch releases of LLVM and we can remove obscure variables like lt_xx_x_x.
The second fact is missed when the minimum LLVM version is raised to LLVM 20 (cf. #145071) and one "fixed in LLVM 20" case can be safely removed (another cannot be removed since it's fixed on LLVM 20.1.1).

It also reorders certain match clauses by the architecture when there's no problems reordering it.

Note that, an LLVM issue on MIPS is fixed on LLVM 20.1.0 and another on AArch64 is fixed on LLVM 20.1.1.
Originally, they are both considered fixed on LLVM 20.1.1 but the author separated them into two cases (so that the MIPS bug checking can be removed).

Related: #146615 (uses similar method to extract LLVM version)
Related: #145071 (follow-up of the minimum LLVM version upgrade to LLVM 20)

r? @workingjubilee

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 19, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2025

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

@a4lg
Copy link
Contributor Author

a4lg commented Sep 19, 2025

Sorry, it seems this function has more problems than it looks and re-investigating now.

@a4lg a4lg force-pushed the codegen-llvm-feature-float-tidying branch from d1c55e3 to 924c9bc Compare September 19, 2025 05:05
@rustbot

This comment has been minimized.

Sign in to view
@a4lg a4lg force-pushed the codegen-llvm-feature-float-tidying branch from 924c9bc to 5ae61db Compare September 19, 2025 05:06
@a4lg
Copy link
Contributor Author

a4lg commented Sep 19, 2025 •
edited
Loading

Re-investigation complete.
I had to separate version checking of version < 20.1.1 into version < 20.1.0 (a MIPS case; equivalent to version < 20) and version < 20.1.1 (an AArch64 case) then perform the rest of the changes.

@a4lg a4lg force-pushed the codegen-llvm-feature-float-tidying branch from 5ae61db to de33db2 Compare September 19, 2025 05:22
@bors

This comment was marked as resolved.

Sign in to view
@petrochenkov
Copy link
Contributor

petrochenkov commented Nov 13, 2025

r? petrochenkov
r=me after rebasing and squashing commits
@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 13, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@a4lg a4lg force-pushed the codegen-llvm-feature-float-tidying branch from de33db2 to 452403d Compare December 20, 2025 07:20
@rustbot

This comment has been minimized.

Sign in to view
a4lg added 3 commits December 20, 2025 07:21
@a4lg
rustc_codegen_llvm: Split "Fixed in LLVM 20" cases
572ca1e 
... in `update_target_reliable_float_cfg`, based on the actual changes.
The AArch64 issue is fixed on LLVM 20.1.1 while the MIPS issue is fixed
on LLVM 20.1.0 (the first LLVM 20 release).

This commit distinguishes two separate cases.
@a4lg
rustc_codegen_llvm: Simplify update_target_reliable_float_cfg
28203e1 
This commit simplifies floating type handling through
`update_target_reliable_float_cfg` based on several facts:

1.  Major changes in behavior normally occurs only
    on the major LLVM upgrade.
2.  The first release of LLVM 20.x.x is 20.1.0.

Due to the first fact, we can normally ignore minor and patch releases
of LLVM and we can remove obscure variables like `lt_xx_x_x` (still,
there is a case where checking for patch version is required).

The second fact is missed when the minimum LLVM version is raised to
LLVM 20 and one "fixed in LLVM 20" case can be safely removed.
@a4lg
rustc_codegen_llvm: Reorder by arch in `update_target_reliable_float_…
42d9f09 
…cfg`

This commit reorders certain match clauses in
`update_target_reliable_float_cfg` by the architecture when
there's no problems reordering it.
@a4lg a4lg force-pushed the codegen-llvm-feature-float-tidying branch from 452403d to 42d9f09 Compare December 20, 2025 07:22
@rustbot
Copy link
Collaborator

rustbot commented Dec 20, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-log-analyzer
Copy link
Collaborator

rust-log-analyzer commented Dec 20, 2025

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
.................................................. (50/144)
F
======== tests/rustdoc-gui/item-info-overflow.goml ========

[ERROR] `tests/rustdoc-gui/item-info-overflow.goml`: item-info-overflow output:
Waiting failed: 30000ms exceeded
stack: TimeoutError: Waiting failed: 30000ms exceeded
    at new WaitTask (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/common/WaitTask.js:50:34)
    at IsolatedWorld.waitForFunction (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/api/Realm.js:25:26)
    at CdpFrame.waitForFunction (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/api/Frame.js:561:43)
    at CdpFrame.<anonymous> (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/util/decorators.js:98:27)
    at CdpPage.waitForFunction (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/puppeteer-core/lib/cjs/puppeteer/api/Page.js:1366:37)
    at runAllCommands (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/browser-ui-test/src/index.js:417:28)
    at async innerRunTestCode (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/browser-ui-test/src/index.js:692:21)
    at async innerRunTests (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/browser-ui-test/src/index.js:630:17)
    at async runTests (/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc-gui/node_modules/browser-ui-test/src/index.js:820:27)



<= doc-ui tests done: 50 succeeded, 94 failed, 0 filtered out

@a4lg
Copy link
Contributor Author

a4lg commented Dec 20, 2025

Resolved the merge conflict (and CI seems flaky).

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 20, 2025
@petrochenkov
Copy link
Contributor

petrochenkov commented Dec 20, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 20, 2025

📌 Commit 42d9f09 has been approved by petrochenkov

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@petrochenkov petrochenkov

Labels

A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@a4lg @rustbot @bors @petrochenkov @rust-log-analyzer @workingjubilee